feat(react): Reactify email-first/Index page #18498
Conversation
| export function useCheckReactEmailFirst() { | ||
| const config = useConfig(); | ||
| // TODO with FXA-11221 (100% prod rollout), remove isInReactExperiment() check | ||
| return config.showReactApp.emailFirstRoutes === true && isInReactExperiment(); |
There was a problem hiding this comment.
Here are the options I debated for this:
- (what I went with) When we roll out React email first to 100%, which requires a 1-line code change, we make it a 2-line change instead and remove an
isInReactExperiment()check - More work to access
fullProdRolloutfrom content-serverreact-app/index.js - Checking query param for
prefillEmail/ all other references, and make follow up to check location state later - Keep the hard navigate and hard reload the React bundle until we clean up later after stabilization
- Take users to React email-first if feature flag is on, even if they aren't in the experiment
LZoog
force-pushed
the
FXA-8636
branch
4 times, most recently
from
c76efc8 to
8801e24
Compare
| @@ -0,0 +1,102 @@ | |||
| /* This Source Code Form is subject to the terms of the Mozilla Public | |||
There was a problem hiding this comment.
Note, this was a port from email-domain-validator in content-server.
|
|
||
| // Since we may perform an async call on initial render that can affect what is rendered, | ||
| // return a spinner on first render. |
There was a problem hiding this comment.
I removed this because very few users (only those hitting signup directly with an email query param, which I think Sync/RPs may do in some cases) should have to wait for this call to complete, and if the account doesn't exist no rerender happens, but if the account does exist then those users will be redirected to /signin. You can test this by going to /[email protected] assuming you've got an account created with that email. It does mean users in this bucket will see signup flashed for a split second (however it's so fast that it's not even noticable locally).
The loading spinner on first render was less noticeable prior to React email-first but it makes the UI feel snappier to remove it now. I see looking now could do a useState for emailStatusChecked and return a loading spinner if not checked yet, but it could be overkill for a very specific edgecase. Depending on the amount of work left here maybe I'll circle back to it.
LZoog
force-pushed
the
FXA-8636
branch
3 times, most recently
from
535be74 to
89dd9f9
Compare
| integration, | ||
| serviceName, | ||
| }: IndexContainerProps & RouteComponentProps) => { | ||
| // TODO, more strict validation for bad oauth params |
There was a problem hiding this comment.
ticket filed for this, will update with next push: FXA-11297
| * License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| 'use strict'; |
There was a problem hiding this comment.
Note for reviewer(s) incl. me - this is extracted from get-index. Did a diffcheck and the only change is a required update to the path for RELEASE on L82
Because: * We are converting our Backbone pages to React This commit: * Adjusts content-server routing to make email-first servable by Express to React * Adds functionality from Backbone email-first to React email-first * Makes React email-first backwards compatible with Backbone until we feel confident in the full prod rollout, e.g. navigate vs hardNavigates * Adjusts stories/tests fixes FXA-8636, fixes FXA-8072
Because:
This commit:
fixes FXA-8636, fixes FXA-8072
TODO:
Non-React routes are not being served by Express properly, except for the Backbone index/email-first screen, e.g. going toThis is fixed./pairin this branch currently 404s. This is preventing me from testing RP redirectLook at/do some stuff in the Backbone Express handler for React (e.g. MX stuff/redirects etc.)InputTextandprefillEmailagain since I've got it as a placeholder but we want it to also be text in the input. Settingvaluemakes it uneditable (still needs doing, see further down)Finish unit testsThings with follow up issues:
Here's some things that could be worth doing maybe as a follow up:
prefillEmailand ensuring the placeholder is a value as well, mentioned above as wellfocusetc. metrics in place here, just the view event and seeing flow stuff carry over into other pagesPreferred way to test
/clearuntil email-first enrolls you and takes you to/?showReactApp=true(which is what will happen for users).You can also use
?forceExperiment=generalizedReactApp&forceExperimentGroup=reactin a pinch but the newisInReactExperiment()in fxa-settings doesn't seem to always work with that. (This seems to add the value to local storage, so not sure off the top of my head why.)Additionally you can go to
/?showReactApp=truedirectly but similarly theisInReactExperiment()check won't work so navigating around back to/will load Backbone.Things to test
/oauth/ticket for this. If you're using 123done and you click "Email-first", you can change the route from/oauth/to/to test